fix: fix incorrect credentials path, create common config logic #231
Conversation
| userdir, err := os.UserConfigDir() | ||
| if err != nil { | ||
| return "", fmt.Errorf("failed to get dbc configuration directory: %v", err) | ||
| } | ||
|
|
||
| orgDirName := "columnar" | ||
| if runtime.GOOS == "windows" || runtime.GOOS == "darwin" { | ||
| orgDirName = "Columnar" | ||
| } | ||
| dbcDirName := "dbc" |
There was a problem hiding this comment.
but you were the one who said I should use XDG_DATA_HOME if it exists.... lol
There was a problem hiding this comment.
Oh right. 🤦 This still fixes the original issue that was reported. Let me think about it and I'll tweak this so it uses XDG_DATA_DIR on Linuxes.
There was a problem hiding this comment.
Which would make this:
- macOS + Windows: os.UserConfigDir()
- Linux: check XDG_DATA_DIR, fallback to os.UserConfigDir()
There was a problem hiding this comment.
it should be XDG_DATA_HOME, there is no XDG_DATA_DIR there is XDG_DATA_DIRS which is a list of paths
There was a problem hiding this comment.
and we should still check XDG_DATA_HOME on macOS too. According to Apple's docs, the equivalent for XDG_DATA_HOME on macOS is ~/Library/
There was a problem hiding this comment.
Oh, that last bit I didn't know.
There was a problem hiding this comment.
Maybe my idea was bad, I'll think about it more when I'm back.
There was a problem hiding this comment.
yea, that's why it was using the logic I laid out. which was separate from the XDG_CONFIG_HOME (os.UserConfigDir) heh
|
Did we come to a consensus on this? |
I wasn't able to find anything solid to go off of. The only thing that recommends a dir like
I think I'm okay with closing this PR unmerged and keeping the current behavior. |
|
I just created a PR with regression tests for the current credentials path behavior: #247. |
2 participants
Closes #230